nitpick: could move type(self) outside this expression, so that it's only evaluated once.

You probably want to call the public ufunc again here, e.g., return getattr(ufunc, method)(*inputs, **kwargs). Otherwise you don't invoke the dispatching mechanism again.

See NDArrayOperatorsMixin for an example implementation.

This should check for isinstance(.., IntegerArray)?

Is there a reason why you use a different sentinel value None rather than Python's standard NotImplemented?

Not a specific reason, just that I didn't return anything in the maybe_dispatch_ufunc_to_dunder_op, which means the result is None (and our own dunder ops will never return None).
But can make it more explicit.

Instead of all this (checking for __array_ufunc__ and then the branch/error message), you could just reinvoke the public ufunc again (as I suggested for IntegerArray). That would also make me more confidence that the proper delegation is happening.

Thanks, that sounds like a better idea :)

Note that not every ufunc returns one result, so you should either handle or explicitly error for the other cases. See NDArrayOperatorsMixin for how to handle this.

Out of curiosity, do you have the same problem with the current implementation of __array_wrap__ ?

To answer my own question: yes, at least for ufuncs that return a scalar:

In [33]: s = pd.Series([1, 2, 3])

In [34]: np.add.reduce(s)
Out[34]: 
0    6
1    6
2    6
dtype: int64

In [35]: pd.__version__
Out[35]: '0.23.4'

But for ufuncs that return multiple values it seems to work with pandas master (wrapping each return element I suppose).

Normally we recommend doing a type check and returning NotImplemented if you see an unknown type . If __array_ufunc__ always tries to do the operation, you can get non-commutative operations, e.g., a + b != b + a.

For example, consider another library like xarray that implements its own version of a pandas.Series. Series + DataArray and DataArray + Series should consistently give either Series, DataArray or an error (probably the safest choice) -- they shouldn't just return a result of the same type as the first argument.

This might even be an issue for Series + DataFrame or np.add(Series, DataFrame) -- you probably want to delegate to the DataFrame in that case, not the Series.

I was thinking somehow: the __array_ufunc__ of the underlying values we call here is already doing the check, so I don't need to do it here again.
But indeed, that will probably mess up such cases.

The problem is that for now the __array_ufunc__ uses our own implementations of all arithmetic/comparison operators, so I should somehow list here everything that all those different ops accept, which might be a bit annoying.

I doubt you need to list types dependent on the operators -- it's probably enough to list the types that can be used in valid operations with pandas.Series.

It's true that you would possibly need to recurse into extension arrays to figure this out all the valid types, and it may not be easy to enumerate all scalar types, e.g., if IPAddressExtensionArray can handle arithmetic with IPAddress, how could pandas.Series know about that?

Yes, I think it are mainly the scalar values which might be hard to list (apart from those, we would probably accept Series, Index, array, and a bunch of array-likes (list, tuple, ..) for the Series case).
But eg a Series with datetimelike values can already handle our own Timestamp, Timedelta and offset objects, numpy scalars, datetime standard library scalars, integer, ..), and as I understand those should all be listed here?
(but anyhow, if we implement an __array_ufunc__ for eg PeriodArray, we would need to do that there as well, so need to make this list in any case)

But what you mention about custom ExtensionArrays is then even another problem, that we don't know what they would accept. By "recurse into extension arrays", would you mean something like checking the ExtensionArray._HANDLED_TYPES (and then expecting EA authors to set this specific property)? Or some other way to inspect?

By "recurse into extension arrays", would you mean something like checking the ExtensionArray._HANDLED_TYPES (and then expecting EA authors to set this specific property)? Or some other way to inspect?

Yeah, that's about the best I can imagine. But I agree, it doesn't seem easy! There's no general way to detect what types another object's __array_ufunc__ method can handle without actually calling it.

It is probably reasonable to trade-off correctness for generality here, but we should be cognizant that we are making this choice. Alternatively, we might maintain an explicit "blacklist" of types which pandas can't handle, though this gets tricky with non-required imports (e.g., dask and xarray objects).

@shoyer in light of the above, how would you define the handled types for an object dtyped Series, in which you in principle can put any python object you want? For numpy arrays, things like addition work element-wise for object dtype, so the following works:

In [101]: class A():
     ...:     def __init__(self, x):
     ...:         self.x = x
     ...:     def __add__(self, other):
     ...:         return A(self.x + other.x)
     ...:     def __repr__(self):
     ...:         return "A(x={})".format(self.x)
     ...:          

In [102]: a = np.array([A(1), A(2)], dtype=object)

In [103]: np.add(a, A(3))
Out[103]: array([A(x=4), A(x=5)], dtype=object)

In [104]: np.add(pd.Series(a), A(3))
Out[104]: 
0    A(x=4)
1    A(x=5)
dtype: object

Doing a more strict check for allowed types, then the above will no longer work.

How is numpy actually dealing with the above? It makes an exception for object dtype?

So two ideas that might be useful here:

• We can make an exception for object dtype, and in that case allow more unknown objects
• We could check for objects that implement __array_ufunc__ themselves? If they have the attribute, and are not known to us (not in our _HANDLED_TYPES list, eg dask or xarray objects), we raise NotImplemented, otherwise we pass through.
  That at least solves the problem for the more known objects that will probably have the interface, but of course not yet for custom container types that do not implement it.

These are good questions. In principle NumPy follows the rules described at http://www.numpy.org/neps/nep-0013-ufunc-overrides.html#behavior-in-combination-with-python-s-binary-operations but I think there is something missing for unknown scalars / object arrays...

See numpy/numpy#12258 for a description of how NumPy does things.

To make sure I understand this thread: @shoyer kicked it off by saying

Normally we recommend doing a type check and returning NotImplemented if you see an unknown type

We can handle the scalar types of EAs by requiring that they implement a _HANDLED_TYPES, which would include their scalar type. Then if all the (unboxed from Series / Index) inputs are in handled types, we proceed.

object-dtype complicates things. Did we decide what to do there? Skip the check?

This is in general the case for pandas containers, I think? (at least in the future if we add ufunc protocol to Index and DataFrame).
So I would make this more general than only Series, that the _HANDLED_TYPES should not include any pandas container types (Series, Index, DataFrame)

Should there be a general item for Series.__array_ufunc__ being added? Although, in principle it should not change any behaviour, as we already supported ufuncs?

This 'scalar' branch, is that this relevant if we raise above for 'reduce' methods? (I don't think any of the non-reduce ufuncs can return a scalar

are we now recommeding setting __array_priority__ ?

blank line before comments